Skip to content

Adds sendtoaddress options comments, subtractfeefromamount#89

Merged
petertodd merged 1 commit intopetertodd:masterfrom
ddrager:sendtoaddress-subtractfeefromamount
Aug 29, 2016
Merged

Adds sendtoaddress options comments, subtractfeefromamount#89
petertodd merged 1 commit intopetertodd:masterfrom
ddrager:sendtoaddress-subtractfeefromamount

Conversation

@ddrager
Copy link
Copy Markdown
Contributor

@ddrager ddrager commented Oct 21, 2015

This pull request fills out newly added option subtractfeefromamount introduced in Bitcoin 0.11. It is used in the sendmany and sendtoaddress calls and will subtract the transaction fee from the amount rather than adding it on top of it. Since the target bitcoin version is 0.11 this should be safe for installs.

Without adding these options it will retain existing functionality. Also had to add the comment parameters for sendtoaddress which were missing.

Ran the runtests.sh for this pull request, not sure what this all means, but the changes are simple and I've tested on my app.

Name                                 Stmts   Miss  Cover
--------------------------------------------------------
bitcoin/__init__.py                     31      8    74%
bitcoin/base58.py                       77      2    97%
bitcoin/bloom.py                       104     14    87%
bitcoin/core/__init__.py               398     39    90%
bitcoin/core/_bignum.py                 69      2    97%
bitcoin/core/key.py                    362     52    86%
bitcoin/core/script.py                 419     18    96%
bitcoin/core/scripteval.py             463     24    95%
bitcoin/core/serialize.py              194     23    88%
bitcoin/messages.py                    345     31    91%
bitcoin/net.py                         131     54    59%
bitcoin/rpc.py                         283    209    26%
bitcoin/signature.py                    30      6    80%
bitcoin/signmessage.py                  40      5    88%
bitcoin/tests/__init__.py                1      0   100%
bitcoin/tests/test_base58.py            32      0   100%
bitcoin/tests/test_bloom.py             61      0   100%
bitcoin/tests/test_checkblock.py        35      6    83%
bitcoin/tests/test_core.py              76      0   100%
bitcoin/tests/test_key.py               19      0   100%
bitcoin/tests/test_messages.py          79      0   100%
bitcoin/tests/test_net.py               45      0   100%
bitcoin/tests/test_rpc.py                5      0   100%
bitcoin/tests/test_script.py           310      0   100%
bitcoin/tests/test_scripteval.py        51      3    94%
bitcoin/tests/test_serialize.py        100      1    99%
bitcoin/tests/test_signmessage.py       44      1    98%
bitcoin/tests/test_transactions.py     100      3    97%
bitcoin/tests/test_wallet.py           127      0   100%
bitcoin/wallet.py                      116      9    92%
setup.py                                 8      0   100%
--------------------------------------------------------
TOTAL                                 4155    510    88%
stats runtests: commands[1] | coverage html
________________________________________________________________________________ summary _________________________________________________________________________________
  reset: commands succeeded
  py27: commands succeeded
  py33: commands succeeded
  py34: commands succeeded
SKIPPED:  py35: InterpreterNotFound: python3.5
SKIPPED:  pypy: InterpreterNotFound: pypy
SKIPPED:  pypy3: InterpreterNotFound: pypy3
  stats: commands succeeded
  congratulations :)

Should be straightforward but let me know if you have any questions.

@petertodd
Copy link
Copy Markdown
Owner

petertodd commented Nov 10, 2015

Hmm, I'm a bit hesitant to remove pre-v0.11 functionality just yet for something as basic as sending funds. In other RPC calls, for example sendrawtransaction, I've maintained backwards compatibility by only including the option if the non-default is set.

OTOH the alternative is something like doing:

def sendtoaddress(self, addr, amount, comment=None, commentto=None, subtractfeefromamount=None):

and then call sendtoaddress with something like:

r = self._call('sendtoaddress', addr, amount, *optional_args)

with optional_args built up from what args are set.

That's kinda complex and seems error prone, particularly going forward. :(

@ddrager
Copy link
Copy Markdown
Contributor Author

ddrager commented Nov 23, 2015

Will take a look at building it like you said to retain pre-0.11 compatibility (I straight up added it because in your Readme.md it said it was targeting 0.11 anyway :) ) as I agree it would be good the retain past version compatibility. Looking for some information on popularity of Bitcoind versions. Will update when I find out more.

@petertodd petertodd merged commit bd200a0 into petertodd:master Aug 29, 2016
petertodd added a commit that referenced this pull request Aug 29, 2016
@petertodd
Merge #89: Adds sendtoaddress options comments, subtractfeefromamount
ee6a12e 
bd200a0 Adds subtractfeefromamount to sendmany and sendtoaddress calls (ddrager)
@petertodd
Copy link
Copy Markdown
Owner

petertodd commented Aug 29, 2016

Merged - we're not longer supporting pre-v0.11 anyway.

rawBit-io pushed a commit to rawBit-io/python-bitcointx that referenced this pull request Nov 6, 2025
@petertodd
Merge petertodd#89: Adds sendtoaddress options comments, subtractfeef…
2492daf 
…romamount

ef9c592 Adds subtractfeefromamount to sendmany and sendtoaddress calls (ddrager)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ddrager @petertodd